-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Add hasScope as a valid SpEL expression to PreAuthorize #18151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add hasScope as a valid SpEL expression to PreAuthorize #18151
Conversation
9d14e8b to
4157ade
Compare
|
Hi, @ngocnhan-tran1996. We want to be careful about adding to the expression root, especially now that it has implications for public interface OAuth2AuthorizationManagerFactory<T> {
default AuthorizationManager<T> hasScope(String scope) {
return OAuth2AuthorizationManagers.hasScope(scope);
}
// ...
}And a default implementation: @Bean
OAuth2AuthorizationManagerFactory<Object> oauth2() {
return new DefaultOAuth2AuthorizationManagerFactory();
}That takes an @Bean
OAuth2AuthorizationManagerFactory<Object> oauth2(AuthorizationManagerFactory<Object> mfa) {
return new OAuth2AuthorizationManagerFactory(mfa);
}And then do: @PreAuthorize("@oauth2.hasScope('message:read')")I like this pattern since it allows for other modules to add their own expressions as well, without needing to change or extend |
cac18db to
e6b1d19
Compare
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
e6b1d19 to
ffc9adf
Compare
|
I’ve made the requested changes. Let me know if anything else is needed. |
jzheaux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @ngocnhan-tran1996! I've left some feedback inline.
| * | ||
| * @param <T> the type of object that the authorization check is being done on | ||
| * @author Ngoc Nhan | ||
| * @since 7.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you please update this to 7.1 since 7 is already released?
| * | ||
| * @param <T> the type of object that the authorization check is being done on | ||
| * @author Ngoc Nhan | ||
| * @since 7.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you please update this to 7.1 since 7 is already released?
| } | ||
| return this.authorizationManagerFactory.hasAnyAuthority(mappedScopes); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you please add hasAllScopes? It can call AuthorizationManagerFactory#hasAllAuthorities
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
Closes: gh-18013